Skip to content

Conversation

@panbingkun
Copy link
Contributor

@panbingkun panbingkun commented Oct 21, 2022

What changes were proposed in this pull request?

This pr replaces TypeCheckFailure by DataTypeMismatch in type checks in the math expressions, includes:

  • hash.scala (HashExpression)
  • mathExpressions.scala (RoundBase)

Why are the changes needed?

Migration onto error classes unifies Spark SQL error messages.

Does this PR introduce any user-facing change?

Yes. The PR changes user-facing error messages.

How was this patch tested?

  • Add new UT
  • Update existed UT
  • Pass GA.

@panbingkun panbingkun changed the title [SPARK-40750][SQL] Migrate type check failures of math expressions onto error classes [WIP][SPARK-40750][SQL] Migrate type check failures of math expressions onto error classes Oct 21, 2022
@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@panbingkun panbingkun changed the title [WIP][SPARK-40750][SQL] Migrate type check failures of math expressions onto error classes [SPARK-40750][SQL] Migrate type check failures of math expressions onto error classes Oct 23, 2022
},
"INVALID_ELEMENT_TYPE" : {
"message" : [
"Input to function <functionName> cannot contain elements of <elementType>. <message>"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's don't pass message, and introduce more specific error class which includes the message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@panbingkun panbingkun requested a review from MaxGekk October 24, 2022 02:04
},
"HASH_MAP_TYPE" : {
"message" : [
"Input to function <functionName> cannot contain elements of MAP. In Spark, same maps may have different hashcode, thus hash expressions are prohibited on MapType elements. To restore previous behavior set \"spark.sql.legacy.allowHashOnMapType\" to \"true\".",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Input to function <functionName> cannot contain elements of MAP. In Spark, same maps may have different hashcode, thus hash expressions are prohibited on MapType elements. To restore previous behavior set \"spark.sql.legacy.allowHashOnMapType\" to \"true\".",
"Input to the function <functionName> cannot contain elements of the \"MAP\" type. In Spark, same maps may have different hashcode, thus hash expressions are prohibited on \"MAP\" elements. To restore previous behavior set \"spark.sql.legacy.allowHashOnMapType\" to \"true\".",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

DataTypeMismatch(
errorSubClass = "WRONG_NUM_PARAMS",
messageParameters = Map(
"functionName" -> prettyName,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, quote the function id by toSQLId()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

"to true.")
DataTypeMismatch(
errorSubClass = "HASH_MAP_TYPE",
messageParameters = Map("functionName" -> prettyName))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quote by toSQLId()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@panbingkun panbingkun requested a review from MaxGekk October 24, 2022 06:00
Copy link
Member

@MaxGekk MaxGekk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for CI.

@MaxGekk
Copy link
Member

MaxGekk commented Oct 24, 2022

+1, LGTM. Merging to master.
Thank you, @panbingkun.

@MaxGekk MaxGekk closed this in 9d2757c Oct 24, 2022
SandishKumarHN pushed a commit to SandishKumarHN/spark that referenced this pull request Dec 12, 2022
…to error classes

### What changes were proposed in this pull request?
This pr replaces TypeCheckFailure by DataTypeMismatch in type checks in the math expressions, includes:
- hash.scala (HashExpression)
- mathExpressions.scala (RoundBase)

### Why are the changes needed?
Migration onto error classes unifies Spark SQL error messages.

### Does this PR introduce _any_ user-facing change?
Yes. The PR changes user-facing error messages.

### How was this patch tested?
- Add new UT
- Update existed UT
- Pass GA.

Closes apache#38332 from panbingkun/SPARK-40750.

Authored-by: panbingkun <pbk1982@gmail.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants